Megaphone API publishing integration #1112
Conversation
kookster
commented
Oct 1, 2024
•
kookster
commented
- Add a new feed type for megaphone
- Save credentials to config
- implement api adapter using faraday for the http library
- megaphone podcast model, retrieving
- megaphone podcast model, persisting
- megaphone episode model, retrieving
- megaphone episode model, persisting
- handle audio changes, with resetting timings
- integrate with callbacks and events to keep in sync?
- Add UI for megaphone feed and config
- Add UI for megaphone episode status
- Update enclosure in feed
- Bug fixes
- More testing of audio replacement and retries
| # encrypt active record attributes | ||
| ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY=0123456789abcdefghijklmnopqrstuv | ||
| ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY=abcdefghijklmnopqrstuv0123456789 | ||
| ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT=abcdefghijk0123456789lmnopqrstuv |
There was a problem hiding this comment.
which is a good reminder I need to deploy values for these to the Infrastructure repo....
| <% integration_status = episode_integration_status(integration, episode) %> | ||
| <div class="col-12 mt-4"> | ||
| <p class="status-text"> | ||
| <strong><%= "#{integration.to_s.titleize} Status" %>:</strong> |
There was a problem hiding this comment.
Nitpick: why not just translate the whole thing? It looks like you were trying to make this generic for any integration, but didn't update the Apple one above this.
There was a problem hiding this comment.
yeah, I was trying not to mess too much with Apple code when I didn't need to, but put in what I thought would be a way to make this code reusable for both used only for megaphone.
I guess I could go ahead and refactor....
|
|
||
| def create_and_update_episodes! | ||
| megaphone_episodes = [] | ||
| private_feed.episodes.unfinished(:megaphone).each do |ep| |
There was a problem hiding this comment.
Do you see the megaphone integration as operating independently from the logic in this abstract episode set operations concern?
With the approach here, we have a simpler/compact interface to the feed's episodes, with no shared episode set concerns between the two existing integrations. But we're lacking visibility into deleted episodes, sorting properties etc. Does that matter?
There was a problem hiding this comment.
The episode set operations are used in the base publisher in a few ways -
episodes_to_sync doesn't work for megaphone, as it filters to only published episodes.
def episodes_to_sync
filter_episodes_to_sync(show.episodes)
end
it is also wrapped with filter_episodes_to_sync, which after querying and instantiating all the episodes rejects from the array based on delivery status, whereas the unfinished method starts with only episodes in need of delivery based on delivery status, which is more efficient, and includes the draft episodes we need to create/update on megaphone.
The archive episodes logic looks closer to what is needed for megaphone, so that method might be useful when that gets implemented.
There was a problem hiding this comment.
I'll also mention, the actual filter for deciding which episodes should be included in the sync is not really using the episode set operations - in the show it's getting the list of feed_ready episodes - so this is not useful for the megaphone integration as a super class method, and the querying to get the episode ids to filter the episodes from the episode set operation does not seem like a better way to do this - I'd rather use scopes and do all the filtering as part of the sql query:
https://github.com/PRX/feeder.prx.org/blob/feat/megaphone/app/models/integrations/base/show.rb#L31
| delete_sync_log | ||
| delete_delivery_status | ||
| self | ||
| end |
There was a problem hiding this comment.
when deleting from megaphone, also need to delete the sync log (otherwise it holds on to the guid, and tries to update a deleted episode), and the delivery status (so it will stop trying to sync when there is nothing in megaphone to sync to)
| @@ -183,7 +199,7 @@ def cuepoints_batch!(cuepoints) | |||
| # validate all the cuepoints about to be created | |||
| cuepoints.all? { |cp| cp.validate!(:create) } | |||
| body = cuepoints.map { |cp| cp.as_json_for_create } | |||
| self.api_response = api.put("podcasts/#{podcast.id}/episodes/#{id}/cuepoints_batch", body) | |||
| self.api_response = api.put_base("episodes/#{id}/cuepoints_batch", body) | |||
There was a problem hiding this comment.
cuepoints for some reason don't include the network in the path, unlike ever other endpoint in the API!
| @@ -303,12 +324,12 @@ def get_media_info(enclosure) | |||
| media_version: nil, | |||
| location: nil, | |||
| size: nil | |||
| } | |||
| }.with_indifferent_access | |||
| @@ -319,13 +340,14 @@ def get_media_info(enclosure) | |||
| raise err | |||
| end | |||
|
|
|||
| def arrangement_version_url(location, media_version, count) | |||
| def arrangement_version_url(location, media_version) | |||
There was a problem hiding this comment.
we don't want the request count in the file name - we really want the media version id to match against
| @@ -24,7 +26,7 @@ def publish! | |||
| def sync_episodes! | |||
| Rails.logger.tagged("Megaphone::Publisher#sync_episodes!") do | |||
| # delete or unpublish episodes we aren't including in the feed anymore | |||
| unpublish_and_delete_episodes! | |||
| delete_episodes! | |||
There was a problem hiding this comment.
megaphone doesn't have an "archived" state - we could unpublish and leave a draft, but that's problematic as well, as those get used for inventory (esp for future episodes).
| def unpublish_and_delete_episodes! | ||
| def delete_episodes! | ||
| megaphone_episodes = [] | ||
| podcast.episodes.with_deleted.where.not(id: private_feed.episodes).each do |ep| |
There was a problem hiding this comment.
It looks like this is skipping the HABTM relation for episodes <-> feeds. It that right? Is there a integration specific reason?
There was a problem hiding this comment.
it is looking at episodes in the podcast, not just the feed, to find episodes removed from the Megaphone feed, which indicates they should be deleted from Megaphone. Those episodes aren't deleted from feeder, but should be deleted from megaphone.
I think there is a case I am missing here though, which is episodes still in the Megaphone feed which have been deleted, I'll add that!
There was a problem hiding this comment.
actually, looking at this, it gets all the podcast episodes (including deleted), and filters out any that are in the megaphone feed and not-deleted (meaning the ones we must keep). So I think this does cover the case of a deleted episode in the megaphone feed, so past-Andrew did cover that case.
